-
-
Notifications
You must be signed in to change notification settings - Fork 262
Correct section name for services in fbtrace config #8500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correct section name for services in fbtrace config #8500
Conversation
| { | ||
| return SectionType::DATABASE_REGEX; | ||
| } | ||
| else if (name == "service") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will break existing trace configs. I strongly disagree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reviewing #8427 I missed that trace-specific routine was put into common class.
It is ConfigFile::Parameter::parseSectionKey() and related enum class SectionType.
I think both must be moved out of ConfigFile::Parameter into TraceCfgReader.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both must be moved out of
ConfigFile::ParameterintoTraceCfgReader.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will break existing trace configs. I strongly disagree.
Existing configurations are using the key 'services'. It was my mistake in the original PR where I incorrectly set the key as 'service'."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reviewing #8427 I missed that trace-specific routine was put into common class. It is
ConfigFile::Parameter::parseSectionKey()and relatedenum class SectionType. I think both must be moved out ofConfigFile::ParameterintoTraceCfgReader.
I have moved the method
|
I don't think that PR with unresolved change request should be merged. |
|
Sorry, but I've seen your approvement and both change requests have been addressed. |
No description provided.